fix: preserve subgraph node size when editing or promoting widgets (FE-853) - #14461
Merged
Conversation
computeSize(out) computes a node's minimum size and, when called with a node's own live size array as out, mutates that array in place - overwriting any larger, user-set size. Three call sites in the subgraph widget-promotion path did this on every promote/demote and on right-side panel mount (which runs pruneDisconnected), silently collapsing resized subgraph nodes back to their minimum footprint. Replace those calls with the existing expandToFitContent() helper, which grows a node to fit new content without shrinking below its current size - the same primitive already used by addInput/addOutput/addWidget. Fixes FE-853
…t setup The three widget-promotion resize tests clicked the enter-subgraph footer button immediately after a manual corner drag, before the browser painted the new layout. Other resize specs (e.g. videoPreview.spec.ts) always wait a frame after resizeFromCorner before reading or acting on the node again; apply the same wait here. The nested-subgraph resize test was missing the @vue-nodes tag and the Comfy.UseNewMenu beforeEach hook that every other test in this file has, so it ran against the classic canvas renderer instead of the Vue node renderer the rest of the suite exercises.
…spec The fixture workflows carry a saved pan/zoom that places nodes partially below the fold on the default 1280x720 Playwright viewport. Growing a node further (or, for the nested-promotion fixture, its unaltered starting size) then pushes its resize handle and the subgraph-enter footer button off-screen, causing the click-intercept timeouts and the no-op resize seen in CI. Fit the view to all nodes right after each workflow load so screen-space interactions land on visible elements.
The Top menu bar's workflow tab strip overlays the canvas at (10, 10), so clicking that fixed coordinate to focus the canvas before pressing the fit-view keybinding intercepted on the tab label and timed out, failing all 6 tests in subgraphResizePreservation.spec.ts. Focusing the canvas element directly sidesteps pointer-event interception entirely.
fitViewToNodes (added to work around topbar interception) legitimately changes canvas zoom away from 1 for the nested-promotion fixture. NodeOperationsHelper.resizeNode combined a screen-space nodePos (NodeReference#getPosition applies pan/zoom) with a raw graph-space nodeSize, so the computed drag points landed off the actual resize handle whenever scale != 1, making the drag a no-op. Scale nodeSize by the canvas zoom before combining it with nodePos.
fitViewToNodes triggered Comfy.Canvas.FitView, which always rescales the canvas to fill 75% of the viewport, changing scale away from 1 even though the fixtures already fit on-screen with a plain pan. That broke NodeOperationsHelper.resizeNode, which combines a screen-space nodePos with a raw graph-space nodeSize and only lands its drag points correctly when scale is 1, and likely destabilized the pixel-ratio heuristic enterSubgraph uses to find the unobstructed part of its footer button. Replace the zoom-to-fit keypress with a direct pan that pins scale to 1 and only translates the canvas offset so every node clears both the viewport edges and the workflow tab strip. This removes the need for resizeNode (or any other pixel-space helper in this suite) to compensate for a non-1 scale, so revert its scale-multiplication workaround.
The nested-promotion resize test clicked subgraph-widget-toggle directly, but that control is intentionally disabled for genuinely linked/promoted widgets (only preview exposures can toggle there). Switch to the same Un-Promote Widget context-menu action used by every other test in this file: enter the outer host's subgraph and un-promote the interior SubgraphNode's widget, which is itself a promotion chain three levels deep (Inner 3 -> Sub 2 -> Sub 1 -> Sub 0).
resizeNode scales the existing size multiplicatively, so 2.5x pushed the outer host node's bottom edge (and the subgraph-enter footer button) ~330px below the 720px viewport, causing enterSubgraph's actionability check to time out. 1.4x keeps the node comfortably on-screen while still satisfying the test's greater-than assertions.
A promoted widget's display is disabled/readonly since its value comes from the subgraph input link. Playwright's plain click() refuses to interact with disabled elements, even for a right-click, so unpromoteWidget's context-menu click was timing out for widgets like 'value_1' in the nested-promotion resize test. The app itself still receives and handles the native contextmenu event on these disabled elements, so bypassing Playwright's actionability check with force exercises the same real user interaction.
Manually resizing a subgraph node and then triggering the widget-editing UI collapses the node back to its computed minimum size, discarding the user's size. Reproduced here via both the 'Edit Subgraph Widgets' selection-toolbox button and promoting an interior widget. Both tests currently fail (node width 506 -> 225), documenting the bug ahead of the fix.
Addresses review feedback: browser_tests/README.md requires setup routines and locator wiring to live in fixtures/helpers/page objects rather than as free-standing functions in a spec. - SubgraphHelper.growNodeByDrag: user-style resize drag returning the model size, so comparisons survive zoom and side-panel layout shifts. - SubgraphEditor.openFromSelectionToolbox: the 'Edit Subgraph Widgets' toolbox button path from the bug report.
Resizing a node by dragging its Vue corner handle isn't subgraph specific, so it belongs alongside the existing resizeNode in nodeOps rather than in SubgraphHelper.
Addresses review feedback: without this the size assertions could hold even if promoteWidget silently became a no-op, letting the regression test pass without exercising the widget-editing path.
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1776 passed, 0 failed · 3 flaky📊 Browser Reports
📦 Bundle: 8.19 MB gzip 🔴 +147 BDetailsSummary
Category Glance App Entry Points — 3.62 kB (baseline 3.62 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.29 MB (baseline 1.29 MB) • 🟢 -48 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 111 kB (baseline 111 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed / 4 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 28.7 kB (baseline 28.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 124 kB (baseline 124 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed / 1 unchanged UI Components — 70 kB (baseline 70 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.44 MB (baseline 3.44 MB) • 🔴 +24 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 386 kB (baseline 386 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 20 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 12.7 MB (baseline 12.7 MB) • ⚪ 0 BBundles that do not match a named category
Status: 70 added / 70 removed / 210 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-31T08:07:23.948Z",
"gitSha": "0e779d739b26045d9feff3bee7a4cc40d5af968c",
"branch": "glary/fe-853-subgraph-resize-unified",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2123.4310000000105,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.504999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 614.1270000000001,
"heapDeltaBytes": 5213988,
"heapUsedBytes": 73109964,
"domNodes": 18,
"jsHeapTotalBytes": 20049920,
"scriptDurationMs": 33.879,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2021.1439999999925,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.415000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 462.125,
"heapDeltaBytes": 4656892,
"heapUsedBytes": 72450332,
"domNodes": 18,
"jsHeapTotalBytes": 20049920,
"scriptDurationMs": 15.968,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2041.667000000018,
"styleRecalcs": 79,
"styleRecalcDurationMs": 45.29900000000001,
"layouts": 12,
"layoutDurationMs": 3.4230000000000005,
"taskDurationMs": 1010.1320000000001,
"heapDeltaBytes": -14499608,
"heapUsedBytes": 53396640,
"domNodes": -278,
"jsHeapTotalBytes": 21749760,
"scriptDurationMs": 137.67499999999998,
"eventListeners": -146,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1912.4700000000416,
"styleRecalcs": 78,
"styleRecalcDurationMs": 43.815,
"layouts": 12,
"layoutDurationMs": 3.9559999999999995,
"taskDurationMs": 936.227,
"heapDeltaBytes": -17429604,
"heapUsedBytes": 50469948,
"domNodes": -281,
"jsHeapTotalBytes": 20439040,
"scriptDurationMs": 128.1,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1737.5450000000114,
"styleRecalcs": 31,
"styleRecalcDurationMs": 20.477999999999994,
"layouts": 6,
"layoutDurationMs": 0.6880000000000001,
"taskDurationMs": 396.02700000000004,
"heapDeltaBytes": 7752144,
"heapUsedBytes": 75435280,
"domNodes": 77,
"jsHeapTotalBytes": 20574208,
"scriptDurationMs": 20.310000000000002,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1725.0639999999748,
"styleRecalcs": 30,
"styleRecalcDurationMs": 15.325999999999999,
"layouts": 6,
"layoutDurationMs": 0.5090000000000001,
"taskDurationMs": 360.51,
"heapDeltaBytes": 7779648,
"heapUsedBytes": 75769272,
"domNodes": 75,
"jsHeapTotalBytes": 19787776,
"scriptDurationMs": 17.134,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 595.454999999987,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.094,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.732,
"heapDeltaBytes": -11988700,
"heapUsedBytes": 55968924,
"domNodes": 18,
"jsHeapTotalBytes": 21098496,
"scriptDurationMs": 59.142,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 605.1049999999805,
"styleRecalcs": 11,
"styleRecalcDurationMs": 7.841999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 403.76099999999997,
"heapDeltaBytes": -12007912,
"heapUsedBytes": 55967136,
"domNodes": 18,
"jsHeapTotalBytes": 22147072,
"scriptDurationMs": 64.79799999999999,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2026.6200000000367,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.861999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 692.806,
"heapDeltaBytes": 7913452,
"heapUsedBytes": 66378848,
"domNodes": -280,
"jsHeapTotalBytes": 4517888,
"scriptDurationMs": 109.808,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2029.0410000000065,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.418999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 696.7319999999999,
"heapDeltaBytes": 7932560,
"heapUsedBytes": 66731736,
"domNodes": -280,
"jsHeapTotalBytes": 4517888,
"scriptDurationMs": 110.406,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2167.041999999981,
"styleRecalcs": 68,
"styleRecalcDurationMs": 14.300000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1310.332,
"heapDeltaBytes": 5294468,
"heapUsedBytes": 65024780,
"domNodes": -282,
"jsHeapTotalBytes": 4743168,
"scriptDurationMs": 426.836,
"eventListeners": -142,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2254.238999999984,
"styleRecalcs": 69,
"styleRecalcDurationMs": 14.395999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1360.7089999999998,
"heapDeltaBytes": 5915336,
"heapUsedBytes": 66291780,
"domNodes": -282,
"jsHeapTotalBytes": 4743168,
"scriptDurationMs": 453.822,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-zoom",
"durationMs": 3162.719999999979,
"styleRecalcs": 65,
"styleRecalcDurationMs": 14.799000000000003,
"layouts": 60,
"layoutDurationMs": 7.348999999999999,
"taskDurationMs": 1470.408,
"heapDeltaBytes": 23453804,
"heapUsedBytes": 84951680,
"domNodes": 12,
"jsHeapTotalBytes": 8388608,
"scriptDurationMs": 531.5999999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3180.8600000000524,
"styleRecalcs": 65,
"styleRecalcDurationMs": 14.351000000000003,
"layouts": 60,
"layoutDurationMs": 7.361999999999998,
"taskDurationMs": 1479.533,
"heapDeltaBytes": 23685996,
"heapUsedBytes": 85138952,
"domNodes": 12,
"jsHeapTotalBytes": 7340032,
"scriptDurationMs": 525.858,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2040.7149999999774,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.419000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 706.1440000000001,
"heapDeltaBytes": 8135472,
"heapUsedBytes": 69213436,
"domNodes": -284,
"jsHeapTotalBytes": 4255744,
"scriptDurationMs": 116.713,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2033.7500000000546,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.919000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 642.7130000000001,
"heapDeltaBytes": 7891524,
"heapUsedBytes": 68636492,
"domNodes": -283,
"jsHeapTotalBytes": 4255744,
"scriptDurationMs": 97.488,
"eventListeners": -144,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 607.6180000000022,
"styleRecalcs": 46,
"styleRecalcDurationMs": 10.900000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 420.62999999999994,
"heapDeltaBytes": -11929128,
"heapUsedBytes": 55910720,
"domNodes": 18,
"jsHeapTotalBytes": 22671360,
"scriptDurationMs": 119.882,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 598.3610000000681,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.091,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 399.134,
"heapDeltaBytes": -12096324,
"heapUsedBytes": 55857352,
"domNodes": 20,
"jsHeapTotalBytes": 22671360,
"scriptDurationMs": 119.90799999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2039.9080000000254,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.705000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 509.5919999999999,
"heapDeltaBytes": -20994460,
"heapUsedBytes": 47011472,
"domNodes": -281,
"jsHeapTotalBytes": 20439040,
"scriptDurationMs": 15.319,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2008.7859999999864,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.498999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 451.604,
"heapDeltaBytes": 4958696,
"heapUsedBytes": 73008176,
"domNodes": 20,
"jsHeapTotalBytes": 20574208,
"scriptDurationMs": 12.772,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1734.8079999999868,
"styleRecalcs": 76,
"styleRecalcDurationMs": 37.318999999999996,
"layouts": 16,
"layoutDurationMs": 4.861000000000001,
"taskDurationMs": 814.36,
"heapDeltaBytes": -19050532,
"heapUsedBytes": 48805836,
"domNodes": -278,
"jsHeapTotalBytes": 21225472,
"scriptDurationMs": 91.347,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1738.5710000000927,
"styleRecalcs": 75,
"styleRecalcDurationMs": 36.169,
"layouts": 16,
"layoutDurationMs": 4.074000000000001,
"taskDurationMs": 813.309,
"heapDeltaBytes": -21696952,
"heapUsedBytes": 46264144,
"domNodes": -280,
"jsHeapTotalBytes": 20439040,
"scriptDurationMs": 91.594,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1385.797000000025,
"styleRecalcs": 18,
"styleRecalcDurationMs": 30.560000000000002,
"layouts": 14,
"layoutDurationMs": 12.738,
"taskDurationMs": 938.9250000000002,
"heapDeltaBytes": 3692924,
"heapUsedBytes": 73105752,
"domNodes": 13673,
"jsHeapTotalBytes": 16515072,
"scriptDurationMs": 33.4,
"eventListeners": 2371,
"totalBlockingTimeMs": 140,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8633.75000000002,
"styleRecalcs": 250,
"styleRecalcDurationMs": 38.961,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4940.733,
"heapDeltaBytes": 18109008,
"heapUsedBytes": 77076760,
"domNodes": -281,
"jsHeapTotalBytes": 8904704,
"scriptDurationMs": 1573.737,
"eventListeners": -126,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8294.401999999991,
"styleRecalcs": 251,
"styleRecalcDurationMs": 39.334999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4485.853999999999,
"heapDeltaBytes": 9030448,
"heapUsedBytes": 69927860,
"domNodes": -279,
"jsHeapTotalBytes": 7593984,
"scriptDurationMs": 1405.1860000000001,
"eventListeners": -126,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 17326.409000000014,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 17287.434,
"heapDeltaBytes": -48007532,
"heapUsedBytes": 160204028,
"domNodes": -8312,
"jsHeapTotalBytes": -13574144,
"scriptDurationMs": 567.677,
"eventListeners": -16385,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.77333333333336,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 16941.093999999906,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16886.933999999997,
"heapDeltaBytes": -39907000,
"heapUsedBytes": 166926808,
"domNodes": -8316,
"jsHeapTotalBytes": -16195584,
"scriptDurationMs": 545.534,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20875.823999999964,
"styleRecalcs": 145,
"styleRecalcDurationMs": 16.91300000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20821.009000000002,
"heapDeltaBytes": -32011156,
"heapUsedBytes": 177032392,
"domNodes": -8312,
"jsHeapTotalBytes": -15478784,
"scriptDurationMs": 897.486,
"eventListeners": -16383,
"totalBlockingTimeMs": 228,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20359.857000000033,
"styleRecalcs": 137,
"styleRecalcDurationMs": 16.061999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20315.918999999998,
"heapDeltaBytes": -30632352,
"heapUsedBytes": 177699532,
"domNodes": -8312,
"jsHeapTotalBytes": -14430208,
"scriptDurationMs": 876.032,
"eventListeners": -16385,
"totalBlockingTimeMs": 149,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 477.98099999999977,
"styleRecalcs": 14,
"styleRecalcDurationMs": 21.159000000000002,
"layouts": 4,
"layoutDurationMs": 1.292,
"taskDurationMs": 130.008,
"heapDeltaBytes": -15904880,
"heapUsedBytes": 50996144,
"domNodes": 130,
"jsHeapTotalBytes": 7991296,
"scriptDurationMs": 11.818999999999999,
"eventListeners": 67,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 110.25200000005952,
"styleRecalcs": 8,
"styleRecalcDurationMs": 14.274999999999999,
"layouts": 1,
"layoutDurationMs": 0.27599999999999997,
"taskDurationMs": 76.084,
"heapDeltaBytes": 2946180,
"heapUsedBytes": 70097460,
"domNodes": 108,
"jsHeapTotalBytes": 2621440,
"scriptDurationMs": 7.000999999999999,
"eventListeners": 33,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
|
@christian-byrne this is the single unified PR against I attempted to add you as assignee and as requested reviewer again; both still return What changed vs the earlier PR chain
#14374, #14415 and #14418 are all superseded and can be closed. I can't post on them — my comment tooling resolves to the newest PR in the repo, which is now this one — but mentioning them in the description above creates a cross-reference in each of their timelines. Re-verified after the rebase onto current
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughSubgraph promotion and editing flows preserve manually resized node dimensions. Rendering expands content without recomputing existing size. Unit and browser tests cover promotion, demotion, pruning, nested subgraphs, and selection-toolbox editing. ChangesSubgraph resize preservation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/rightSidePanel/subgraph/SubgraphEditor.vue (1)
230-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider reusing the module-level
refreshPromotedWidgetRenderinginstead of duplicating it here.This function is structurally identical to
refreshPromotedWidgetRenderinginsrc/core/graph/subgraph/promotionUtils.ts(Lines 260-265): both callexpandToFitContent(),setDirtyCanvas(true, true), and dirty the canvas store. This PR had to apply the identical one-line fix in both places, which shows the duplication carries real drift risk: a future change to one copy can silently miss the other.Export the
promotionUtils.tsversion and call it here with[node]instead of keeping a local copy.♻️ Proposed refactor
-function refreshPromotedWidgetRendering() { - const node = activeNode.value - if (!node) return - - node.expandToFitContent() - node.setDirtyCanvas(true, true) - canvasStore.canvas?.setDirty(true, true) -} +function refreshPromotedWidgetRendering() { + const node = activeNode.value + if (!node) return + refreshPromotedWidgetRenderingForParents([node]) +}And in
promotionUtils.ts, addexporttorefreshPromotedWidgetRendering(renaming if desired to avoid a naming collision with the local wrapper).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/rightSidePanel/subgraph/SubgraphEditor.vue` around lines 230 - 237, Remove the local refreshPromotedWidgetRendering implementation in SubgraphEditor.vue and import the shared function from promotionUtils.ts. Export the existing promotionUtils.ts function, then invoke it with the active node as the [node] argument while preserving the current no-active-node guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@browser_tests/tests/subgraph/subgraphResizePreservation.spec.ts`:
- Around line 100-118: Add await comfyPage.nextFrame() immediately after each
resizeFromCorner() call in the affected tests, before capturing boundingBox(),
including the cases around the resizedBox and afterBox measurements. Match the
existing sibling-test synchronization pattern while leaving the assertions
unchanged.
---
Outside diff comments:
In `@src/components/rightSidePanel/subgraph/SubgraphEditor.vue`:
- Around line 230-237: Remove the local refreshPromotedWidgetRendering
implementation in SubgraphEditor.vue and import the shared function from
promotionUtils.ts. Export the existing promotionUtils.ts function, then invoke
it with the active node as the [node] argument while preserving the current
no-active-node guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 658986d1-14d1-41eb-a86f-1627f936b346
📒 Files selected for processing (10)
browser_tests/fixtures/components/ContextMenu.tsbrowser_tests/fixtures/components/SubgraphEditor.tsbrowser_tests/fixtures/helpers/NodeOperationsHelper.tsbrowser_tests/fixtures/helpers/SubgraphHelper.tsbrowser_tests/tests/subgraph/subgraphEditWidgetsResize.spec.tsbrowser_tests/tests/subgraph/subgraphResizePreservation.spec.tssrc/components/rightSidePanel/subgraph/SubgraphEditor.test.tssrc/components/rightSidePanel/subgraph/SubgraphEditor.vuesrc/core/graph/subgraph/promotionUtils.test.tssrc/core/graph/subgraph/promotionUtils.ts
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #14461 +/- ##
==========================================
+ Coverage 78.24% 78.61% +0.37%
==========================================
Files 1767 1767
Lines 110746 102864 -7882
Branches 35074 32373 -2701
==========================================
- Hits 86655 80870 -5785
+ Misses 23544 21572 -1972
+ Partials 547 422 -125
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 191 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Addresses CodeRabbit review on #14461: - SubgraphEditor.vue duplicated promotionUtils' refreshPromotedWidgetRendering body, which is why the FE-853 one-line fix had to be applied twice. Export the shared helper and delegate to it so the two cannot drift again. - Two tests in subgraphResizePreservation read boundingBox() straight after resizeFromCorner() while their siblings await nextFrame() first, risking an unsettled layout read. Added the missing waits.
|
Both CodeRabbit findings addressed in daeffec. 1. Duplicated The drift argument is the convincing part: this PR is itself the evidence, since the FE-853 one-line fix had to be applied to both copies independently. A third call site or a future change would very likely miss one.
function refreshPromotedWidgetRendering() {
const node = activeNode.value
if (!node) return
refreshPromotedWidgetRenderingForNodes([node])
}I kept the thin local wrapper rather than inlining I did not touch the third 2. Missing Re-verified after these changes
The unit and e2e coverage over both edited functions is what gives me confidence the dedupe is behavior-preserving rather than just type-clean. |
DrJKL
marked this pull request as ready for review
July 31, 2026 05:10
dosubot
Bot
added
the
size:M
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
christian-byrne
previously approved these changes
Jul 31, 2026
| * handle. The returned size is read from the graph model, not a DOM bounding | ||
| * box, so it stays comparable across zoom and side-panel layout changes. | ||
| */ | ||
| async growNodeByDrag( |
Contributor
There was a problem hiding this comment.
Should this be using expects or just throws?
There was a problem hiding this comment.
Throws — you're right, changed in c74a4e9.
Assertions in a fixture helper attribute failures to the fixture rather than the test, and blur the line between "the helper couldn't do its job" and "the behaviour under test is wrong". Both checks here are the former, so they're now throws:
const [nodeRef] = await this.getNodeRefsByTitle(title)
if (!nodeRef) throw new Error(`No node titled "${title}" on the canvas`)
...
if (size.width <= sizeBefore.width || size.height <= sizeBefore.height) {
throw new Error(
`Resize drag did not enlarge "${title}": ${sizeBefore.width}x${sizeBefore.height} -> ${size.width}x${size.height}`
)
}Collapsed the two size expects into a single throw that reports before and after, which is more useful than the two separate messages were. Also dropped the now-unused expect import from the file.
| widgetName: string | ||
| ): Promise<void> { | ||
| const widget = nodeLocator.getByLabel(widgetName, { exact: true }) | ||
| // A promoted widget's own display is disabled/readonly (its value comes |
Contributor
There was a problem hiding this comment.
Can we make this expressed in code rather than comments?
There was a problem hiding this comment.
Done in c74a4e9 — and chasing this turned up a lint violation, so thanks for pushing on it.
The comment existed to explain a force: true boolean flag. Replaced both with a named method, so the call site reads for itself and openFor goes back to its original signature:
await this.comfyPage.contextMenu
.openForDisabledElement(widget)
.then((m) => m.clickMenuItemExact(`Un-Promote Widget: ${widgetName}`))While implementing it, the pre-commit hook rejected { force: true } under playwright/no-force-option, which is "error" in .oxlintrc.json. The original code slipped past it only because it passed force as a variable rather than a literal. So rather than reintroduce a forced click, openForDisabledElement dispatches the event directly:
async openForDisabledElement(locator: Locator): Promise<this> {
await locator.dispatchEvent('contextmenu', { button: 2 })
await expect(this.anyMenu).toBeVisible()
return this
}That reaches the same @contextmenu handler the widgets bind in NodeWidgets.vue, so it exercises the real path, needs no explanatory comment, and is lint-clean.
Verified against the two tests that go through unpromoteWidget — "Un-promoting a widget preserves a user-resized subgraph node" and the nested "Demoting a nested promotion…" — plus the rest of the file: 8/8 passing.
| * by the "Edit Subgraph Widgets" selection-toolbox button or by promoting a | ||
| * widget from inside the subgraph — discarding the size the user set. | ||
| * | ||
| * https://linear.app/comfyorg/issue/FE-853 |
Contributor
There was a problem hiding this comment.
No need to link linear. Can be shorter comment. Prefer playwright test annotation over docstring IMO
There was a problem hiding this comment.
Agreed on all three — done in c74a4e9. The 8-line docstring and the Linear URL are gone, replaced by a Playwright annotation on the describe:
test.describe(
'Subgraph node size across widget editing',
{
tag: ['@subgraph', '@node', '@widget', '@vue-nodes'],
annotation: {
type: 'issue',
description:
'FE-853: widget editing collapsed a user-resized subgraph node to its minimum size'
}
},
() => {Better than the docstring in that it surfaces in the HTML report and via TestCase.annotations, instead of only being visible to someone opening the file.
Used the declarative annotation field in the details object rather than test.info().annotations.push(...) (the pattern in workflowPersistence.spec.ts) since it's static metadata — confirmed supported on the pinned Playwright 1.61.1.
Kept the issue key in the description but not the URL, per your point.
| isRecommendedWidget, | ||
| promoteWidget, | ||
| pruneDisconnected, | ||
| refreshPromotedWidgetRendering as refreshPromotedWidgetRenderingForNodes, |
Contributor
There was a problem hiding this comment.
Avoid alias importing
There was a problem hiding this comment.
Alias removed in c74a4e9. The import is now plain:
refreshPromotedWidgetRendering,The alias only existed because the local wrapper had the same name. Renamed that wrapper to refreshActiveNodeRendering, which is more accurate anyway — its whole job is resolving activeNode and delegating:
function refreshActiveNodeRendering() {
const node = activeNode.value
if (!node) return
refreshPromotedWidgetRendering([node])
}Updated its three call sites in the file. Confirmed by the 5 subgraphPromotedWidgetPanel e2e tests and the 60 unit tests over SubgraphEditor / promotionUtils.
Open
4 tasks
christian-byrne
approved these changes
Jul 31, 2026
DrJKL
added
needs-backport
Member
Member
github-actions
Bot
removed
the
needs-backport
2 tasks
2 tasks
This was referenced Aug 1, 2026
Regression: Subgraph exposed widgets eye icons broken in frontend 1.47.11 (worked in 1.45.21)
#14536
Closed
This was referenced Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Created by the Glary-Bot Agent
Single unified PR for FE-853 — the fix plus full regression coverage, targeting
main.Supersedes #14374, #14415 and #14418, all of which can be closed. This branch contains the fix originally proposed in #14374 (authorship preserved) with the regression test from #14415 folded in, rebased onto current
main.The bug
Resize a subgraph node, then touch the widget-editing UI, and the node snaps back to its computed minimum size — silently discarding the size you set. Reported in #bug-dump via the "Edit Subgraph Widgets" button; FE-853 tracks the same collapse when promoting or un-promoting a widget.
Measured against
mainbefore the fix:The fix
Three call sites refreshed a subgraph host node's rendering with
computeSize(node.size).computeSizereturns the minimum size for the node's content and those calls assigned it back, so every refresh clamped the node down and destroyed any larger user-set size.Replaced with
expandToFitContent(), which grows the node when content requires more room but never shrinks it:src/components/rightSidePanel/subgraph/SubgraphEditor.vue—refreshPromotedWidgetRendering(), the path the "Edit Subgraph Widgets" button hitssrc/core/graph/subgraph/promotionUtils.ts—refreshPromotedWidgetRendering()(promote/demote) andpromoteRecommendedWidgets()Regression coverage
subgraphEditWidgetsResize.spec.ts(2 tests) — approaches the bug from the user-reported entry point:subgraphResizePreservation.spec.ts(6 tests) — promote, un-promote, panel open, multi-widget sequences, resize-still-works-after-promotion, and a nested-subgraph host.Unit tests (60) across
promotionUtils.test.tsandSubgraphEditor.test.ts.Sizes are read from the graph model rather than DOM bounding boxes, so assertions aren't confounded by canvas zoom or by the canvas shrinking when the side panel opens. The promotion test asserts
stepsactually reached the host node before checking size, so it can't pass vacuously if promotion silently became a no-op.Supporting fixtures:
NodeOperationsHelper.growNodeByDrag()(user-style resize drag returning model-space size, beside the existing canvas-basedresizeNode) andSubgraphEditor.openFromSelectionToolbox()(toolbox-button path, complementing the context-menuensureOpen()).Verification
The tests were confirmed to detect the regression rather than pass incidentally, by reverting the fix in place on this branch:
srcstatesubgraphEditWidgetsResizeexpandToFitContent()computeSize(node.size)expected 506.0625, received 225In the reverted run the failing assertion is the width check, not the promotion guard.
Against current
mainafter rebase: 8 e2e passed, 60 unit passed, andpnpm typecheck,pnpm typecheck:browser, ESLint, oxlint, oxfmt--checkandpnpm knipall clean.Review history carried over
growNodeByDragbelongs outsideSubgraphHelper→ it lives inNodeOperationsHelper;SubgraphHelperis unchanged apart from fix: preserve subgraph node size across widget promotion #14374's own edit.getPromotedWidgetOrder()guard. CodeRabbit approved test: add failing regression test for subgraph resize loss (FE-853) #14415 and test: fold FE-853 Edit Subgraph Widgets regression test into the fix #14418.Screenshots